Skip to content

bugfix(render2d): Flush batched 2D draws before they overflow the dynamic buffers - #3487

Open
tintinhamans wants to merge 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/fix/render2d-batch-overflow
Open

tintinhamans wants to merge 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/fix/render2d-batch-overflow

Conversation

@tintinhamans

@tintinhamans tintinhamans commented Oct 12, 2026 •

Copy link
Copy Markdown

Render2DClass::Render passes Indices.Count() to DynamicIBAccessClass, which takes an unsigned short. 65538 indices become 2, the index buffer is locked for 2, and the copy loop writes all 65538.

Before batching, W3DDisplay called Render after every draw call. With batching, setup2DRenderState previously only flushed when the texture, mode or grayscale changed, so identical draws could accumulate beyond the dynamic buffer limits.

Drawable::drawAmmo hits this: it draws one pip image per round in the clip, and ClipSize is a plain INI::parseInt. A weapon with ShowsAmmoPips and a clip of 10923 produces 65538 indices, and selecting or hovering the unit can write up to 128 KB past the buffer.

DynamicVBAccessClass and DynamicIBAccessClass now define MAX_VERTEX_COUNT and MAX_INDEX_COUNT. Render2DClass::Can_Add checks whether the batch has room for the requested vertices and indices.

Each draw supplies its actual geometry counts to setup2DRenderState, including rotated images and each clock branch. The batch flushes before the next draw would exceed either limit, rather than at half capacity.

Render keeps a runtime guard against an unsafe buffer copy, because WorldBuilder's DrawObject calls it on its own Render2DClass without this flush.

The similar narrowing in drawRiverWater is handled separately.


AI Disclosure: issue identified by Codex Security Cloud and fix implemented with assistance from Opus 5.5

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The Generals and GeneralsMD 2D renderers now check vertex and index capacity. W3DDisplay supplies expected draw counts during batch setup and flushes and reinitializes a batch when it cannot fit the upcoming draw.

Changes

2D renderer capacity checks

Layer / File(s) Summary
Capacity contract and render guard
Core/Libraries/Source/WWVegas/WW3D2/dx8vertexbuffer.h, Generals/Code/Libraries/Source/WWVegas/WW3D2/render2d.*, GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.*
DynamicVBAccessClass defines a vertex-count limit. Both Render2DClass implementations add Can_Add and return without rendering when current geometry exceeds the buffer limits.
Batch capacity check
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DDisplay.h, Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
setup2DRenderState receives expected vertex and index counts. Primitive, clock, image, and video-buffer draw paths provide capacity estimates. The batch is flushed and reinitialized when it cannot fit the upcoming draw.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

Merge Risk: Minimal · up to c588b

No actionable merge-blocking risk remains identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: flushing batched 2D draws before dynamic-buffer overflow.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description directly explains the buffer-overflow risk and the changes that add capacity checks and preemptive batch flushing.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were found.

Summary

The PR flushes batched 2D draws before either dynamic buffer exceeds its count limit.

  • 2D batches flush before their geometry exceeds buffer limits.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Next 2D draw] --> B{Same draw state and enough room?}
    B -->|Yes| E[Append geometry]
    B -->|No| C[Render and reset current batch]
    C --> D[Set requested draw state]
    D --> E
    E --> F[Flush at batch end or before next incompatible draw]
Loading

Reviews (7) · Last reviewed commit: "fix(render2d): flush batched draws befor..." · Reviewed by Greptile

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
Comment thread GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.h Outdated
@tintinhamans
tintinhamans force-pushed the arctic/fix/render2d-batch-overflow branch from 1bdda9d to 1714002 Compare October 13, 2026 07:55
Comment thread GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.cpp Outdated
@tintinhamans tintinhamans added Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Memory Is memory related Stability Concerns stability of the runtime ThisProject The issue was introduced by this project, or this task is specific to this project Bug Something is not working right, typically is user facing Performance Is a performance concern and removed Stability Concerns stability of the runtime labels Oct 13, 2026

@Mauller Mauller left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread Generals/Code/Libraries/Source/WWVegas/WW3D2/render2d.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp Outdated
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp Outdated
@tintinhamans
tintinhamans force-pushed the arctic/fix/render2d-batch-overflow branch from 1714002 to fd459dd Compare October 18, 2026 02:30
@tintinhamans
tintinhamans requested review from Mauller and xezon October 18, 2026 02:30
@tintinhamans
tintinhamans force-pushed the arctic/fix/render2d-batch-overflow branch 2 times, most recently from 6b298b2 to d13fd9a Compare October 23, 2026 07:37
Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp Outdated
@tintinhamans
tintinhamans force-pushed the arctic/fix/render2d-batch-overflow branch from d13fd9a to 13462fe Compare October 27, 2026 10:19
UnsignedInt lineColor )
{
setup2DRenderState(nullptr, DRAW_IMAGE_ALPHA, FALSE);
setup2DRenderState(nullptr, DRAW_IMAGE_ALPHA, FALSE, 4, 6);

@Mauller Mauller Oct 28, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might help to have some constants for the 4 and 6 instead of magic numbers.

Probably want something like verticesPerQuad etc.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, added named constants for the quad and triangle counts.

@tintinhamans
tintinhamans force-pushed the arctic/fix/render2d-batch-overflow branch from 13462fe to c588bcf Compare October 29, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker Memory Is memory related Performance Is a performance concern ThisProject The issue was introduced by this project, or this task is specific to this project ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants